Order code actions #1078
Order code actions #1078
Conversation
|
|
||
| namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 | ||
| { | ||
| internal class Graph<T> |
There was a problem hiding this comment.
Yes, this is taken from Extension Orderer used by Roslyn and modified a bit for the current case.
There was a problem hiding this comment.
Ok, we might want to add a comment stating as such.
There was a problem hiding this comment.
@DustinCampbell can we get that API opened up publicly? 😀
| } | ||
| } | ||
|
|
||
| var graph = Graph<CodeFixProvider>.GetGraph(nodesList); | ||
| return graph.TopologicalSort(); |
There was a problem hiding this comment.
Can we cache this? (Eg, does the total set of CodeFixProviders change)
| @@ -23,6 +23,8 @@ public abstract class BaseCodeActionService<TRequest, TResponse> : IRequestHandl | |||
| { | |||
| protected readonly OmniSharpWorkspace Workspace; | |||
| protected readonly IEnumerable<ICodeActionProvider> Providers; | |||
| protected List<CodeFixProvider> OrderedCodeFixProviders; | |||
There was a problem hiding this comment.
You can make this a Lazy<IEnumerable<CodeFixProvider> and initialize it like
_lazyFixProviders = new Lazy(() => GetOrderedProviders)```
That will let you avoid some of the crazy exception handling below.
| private void SetOrderedProviders() | ||
| { | ||
| try | ||
| { |
There was a problem hiding this comment.
Why is this in a try/catch? What are you expecting to fail?
There was a problem hiding this comment.
the GetSortedCodeFixProviders() will return an exception if there are cycles in the graph. This try catch will handle the exception and return the unordered list if that exception occurs
There was a problem hiding this comment.
@rchande your last comments seem to be resolved. Change looks good to me. 👍
|
green! |
Fixes : #758
To order code actions we can order the providers based on the ExtensionOrder attribute in roslyn.There are two types of providers - CodeFixProvider and CodeRefactoringProvider. For each of these categories we perform the following on a list of providers :
Please review @DustinCampbell @TheRealPiotrP @rchande @filipw